Skip to content

Support async signing of interactive-tx initial commitment signatures #4355

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
TheBlueMatt merged 1 commit into from
Feb 6, 2026
Merged

Support async signing of interactive-tx initial commitment signatures #4355

TheBlueMatt merged 1 commit into from
Feb 6, 2026
+237 −51

Conversation

@wpaulino
Copy link
Contributor

@wpaulino wpaulino commented Jan 28, 2026 

This commit allows for an async signer to immediately return upon a call
to `EcdsaChannelSigner::sign_counterparty_commitment` for the initial
commitment signatures of an interactively funded transaction, such that
they can call back in via `ChannelManager::signer_unblocked` once the
signatures are ready. This is done for both splices and dual-funded
channels, though note that the latter still require more work to be
integrated. Since `tx_signatures` must be sent only after exchanging
`commitment_signed`, we make sure to hold them back if they're ready to
be sent until our `commitment_signed` is also ready.

Depends on #4336.

@wpaulino wpaulino added this to the 0.3 milestone Jan 28, 2026
@wpaulino wpaulino self-assigned this Jan 28, 2026
@ldk-reviews-bot
Copy link

ldk-reviews-bot commented Jan 28, 2026 •
edited
Loading

👋 Thanks for assigning @TheBlueMatt as a reviewer!
I'll wait for their review and will help manage the review process.
Once they submit their review, I'll check if a second reviewer would be helpful.

@codecov
Copy link

codecov bot commented Jan 28, 2026 •
edited
Loading

Codecov Report

❌ Patch coverage is 86.27451% with 14 lines in your changes missing coverage. Please review.
✅ Project coverage is 86.03%. Comparing base (dde2c82) to head (ee74209).
⚠️ Report is 13 commits behind head on main.

Files with missing lines Patch % Lines
lightning/src/ln/channel.rs 83.33% 11 Missing and 3 partials ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #4355      +/-   ##
==========================================
+ Coverage   86.00%   86.03%   +0.02%     
==========================================
  Files         156      156              
  Lines      102857   102909      +52     
  Branches   102857   102909      +52     
==========================================
+ Hits        88465    88538      +73     
+ Misses      11883    11865      -18     
+ Partials     2509     2506       -3
Flag Coverage Δ
tests 86.03% <86.27%> (+0.02%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

TheBlueMatt
TheBlueMatt previously approved these changes Jan 28, 2026
View reviewed changes
Copy link
Collaborator

@TheBlueMatt TheBlueMatt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool! Easy enough. We should ask claude to add async signing to the chanmon_consistency fuzzer as a followup.

This was linked to issues Jan 29, 2026
async signing completion #3489
Closed
Async + Fallible Signing #2088
Closed
@TheBlueMatt
Copy link
Collaborator

TheBlueMatt commented Jan 29, 2026

Marked this as fixing #3489 and #2088 though we should probably see if we need to address #3109 (review) as well.

@wpaulino
Copy link
Contributor Author

wpaulino commented Jan 29, 2026

Marked this as fixing #3489 and #2088

There's one more case for dual-funding that still needs to be addressed -- the first_per_commitment_point in open_channel2.

@TheBlueMatt
Copy link
Collaborator

TheBlueMatt commented Jan 29, 2026

Sure but we don't actually do dual-funding yet, so that's just a part of doing dual-funding now :)

@wpaulino wpaulino force-pushed the splice-async-signing branch from 9933019 to eb9fc0c Compare January 30, 2026 03:32
@ldk-reviews-bot
Copy link

ldk-reviews-bot commented Jan 31, 2026

🔔 1st Reminder

Hey @jkczyz! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

@ldk-reviews-bot
Copy link

ldk-reviews-bot commented Feb 2, 2026

🔔 2nd Reminder

Hey @jkczyz! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

@ldk-reviews-bot
Copy link

ldk-reviews-bot commented Feb 4, 2026

🔔 3rd Reminder

Hey @jkczyz! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

jkczyz
jkczyz reviewed Feb 5, 2026
View reviewed changes
@wpaulino wpaulino force-pushed the splice-async-signing branch from eb9fc0c to 252545c Compare February 5, 2026 17:57
@wpaulino
Support async signing of interactive-tx initial commitment signatures
ee74209 
This commit allows for an async signer to immediately return upon a call
to `EcdsaChannelSigner::sign_counterparty_commitment` for the initial
commitment signatures of an interactively funded transaction, such that
they can call back in via `ChannelManager::signer_unblocked` once the
signatures are ready. This is done for both splices and dual-funded
channels, though note that the latter still require more work to be
integrated. Since `tx_signatures` must be sent only after exchanging
`commitment_signed`, we make sure to hold them back if they're ready to
be sent until our `commitment_signed` is also ready.
@wpaulino wpaulino force-pushed the splice-async-signing branch from 252545c to ee74209 Compare February 5, 2026 18:01
@wpaulino
Copy link
Contributor Author

wpaulino commented Feb 5, 2026

Had to rebase due to small conflict after #4311

@wpaulino wpaulino requested a review from jkczyz February 5, 2026 18:02
@wpaulino wpaulino requested a review from TheBlueMatt February 5, 2026 20:29
@TheBlueMatt TheBlueMatt merged commit 372ba65 into lightningdevkit:main Feb 6, 2026
19 checks passed
@wpaulino wpaulino deleted the splice-async-signing branch February 6, 2026 16:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@TheBlueMatt TheBlueMatt TheBlueMatt approved these changes

@jkczyz jkczyz jkczyz approved these changes

Assignees

@wpaulino wpaulino

Labels

None yet

Projects

Weekly Goals
Status: Done

Milestone

0.3

Development

Successfully merging this pull request may close these issues.

async signing completion Async + Fallible Signing

4 participants

@wpaulino @ldk-reviews-bot @TheBlueMatt @jkczyz